Topological Sorting and Sequenced Execution #26

dbwiddis · 2023-09-08T16:41:53Z

Description

Uses Topological Sorting to parse a JSON graph description of nodes and edges into a sequence of processes. Uses asynchronous execution with CompletableFutures to execute the processes.

Example input:

{
    "sequence": {
        "nodes": [
            {
                "id": "fetch_model"
            },
            {
                "id": "create_ingest_pipeline"
            },
            {
                "id": "create_search_pipeline"
            },
            {
                "id": "create_neural_search_index"
            }
        ],
        "edges": [
            {
                "source": "fetch_model",
                "dest": "create_ingest_pipeline"
            },
            {
                "source": "fetch_model",
                "dest": "create_search_pipeline"
            },
            {
                "source": "create_ingest_pipeline",
                "dest": "create_neural_search_index"
            },
            {
                "source": "create_search_pipeline",
                "dest": "create_neural_search_index"
            }
        ]
    }
}

Example output showing sequencing with parallel paths:

17:22:34.597 INFO  o.o.f.demo.Demo - Parsing graph to sequence...
17:22:34.634 DEBUG o.o.f.template.TemplateParser - Start node(s): [fetch_model]
17:22:34.635 DEBUG o.o.f.template.TemplateParser - Execution sequence: [fetch_model, create_search_pipeline, create_ingest_pipeline, create_neural_search_index]
17:22:34.636 INFO  o.o.f.demo.Demo - Queueing process [fetch_model]. Can start immediately!
17:22:34.638 INFO  o.o.f.demo.Demo - Queueing process [create_search_pipeline]. Must wait for [fetch_model] to complete first.
17:22:34.638 DEBUG o.o.f.template.ProcessNode - >>> Starting fetch_model.
17:22:34.638 INFO  o.o.f.demo.Demo - Queueing process [create_ingest_pipeline]. Must wait for [fetch_model] to complete first.
17:22:34.639 INFO  o.o.f.demo.Demo - Queueing process [create_neural_search_index]. Must wait for [create_ingest_pipeline, create_search_pipeline] to complete first.
17:22:37.640 DEBUG o.o.f.template.ProcessNode - <<< Completed fetch_model
17:22:37.640 DEBUG o.o.f.template.ProcessNode - >>> Starting create_ingest_pipeline.
17:22:37.640 DEBUG o.o.f.template.ProcessNode - >>> Starting create_search_pipeline.
17:22:40.644 DEBUG o.o.f.template.ProcessNode - <<< Completed create_ingest_pipeline
17:22:42.644 DEBUG o.o.f.template.ProcessNode - <<< Completed create_search_pipeline
17:22:42.644 DEBUG o.o.f.template.ProcessNode - >>> Starting create_neural_search_index.
17:22:44.648 DEBUG o.o.f.template.ProcessNode - <<< Completed create_neural_search_index
17:22:44.648 INFO  o.o.f.demo.Demo - All done!

Example showing initialized data:

{
    "sequence": {
        "nodes": [
            {
                "id": "create_index",
                "index_name": "demo"
            },
            {
                "id": "create_another_index",
                "index_name": "second_demo"
            }
        ],
        "edges": [
            {
                "source": "create_index",
                "dest": "create_another_index"
            }
        ]
    }
}

And the processing:

10:37:55.530 INFO  o.o.f.demo.DataDemo - Parsing graph to sequence...
10:37:55.594 DEBUG o.o.f.template.TemplateParser - Start node(s): [create_index]
10:37:55.595 DEBUG o.o.f.template.TemplateParser - Execution sequence: [create_index, create_another_index]
10:37:55.595 INFO  o.o.f.demo.DataDemo - Queueing process [create_index]. Can start immediately!
10:37:55.599 INFO  o.o.f.demo.DataDemo - Queueing process [create_another_index]. Must wait for [create_index] to complete first.
10:37:55.599 DEBUG o.o.f.template.ProcessNode - >>> Starting create_index.
10:37:55.684 DEBUG o.o.f.demo.CreateIndexWorkflowStep - Initialization sent params: {index=demo}, content: {aliases=[], settings={}, mappings={}}
10:37:57.699 DEBUG o.o.f.template.ProcessNode - <<< Completed create_index
10:37:57.699 DEBUG o.o.f.template.ProcessNode - >>> Starting create_another_index.
10:37:57.700 DEBUG o.o.f.demo.CreateIndexWorkflowStep - Initialization sent params: {index=second_demo}, content: {aliases=[], settings={}, mappings={}}
10:37:57.700 DEBUG o.o.f.demo.CreateIndexWorkflowStep - Previous step sent params: {}, content: {index=demo}
10:37:59.705 DEBUG o.o.f.template.ProcessNode - <<< Completed create_another_index
10:37:59.705 INFO  o.o.f.demo.DataDemo - All done!

Issues Resolved

Fixes #17
Fixes #19
Fixes #29
Fixes #39

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

owaiskazi19

First pass review

src/main/java/org/opensearch/flowframework/template/ProcessNode.java

src/main/java/org/opensearch/flowframework/template/TemplateParser.java

Signed-off-by: Daniel Widdis <[email protected]>

src/main/java/org/opensearch/flowframework/template/ProcessNode.java

codecov · 2023-09-16T01:02:04Z

Codecov Report

Merging #26 (d1cbcbd) into main (0f0b65d) will increase coverage by 74.26%.
The diff coverage is 76.51%.

@@             Coverage Diff              @@
##              main      #26       +/-   ##
============================================
+ Coverage     0.00%   74.26%   +74.26%     
- Complexity       0       42       +42     
============================================
  Files            2        6        +4     
  Lines            4      136      +132     
  Branches         0       16       +16     
============================================
+ Hits             0      101      +101     
- Misses           4       25       +21     
- Partials         0       10       +10

Files Changed	Coverage Δ
...opensearch/flowframework/template/ProcessNode.java	`60.71% <60.71%> (ø)`
...ch/flowframework/template/ProcessSequenceEdge.java	`69.23% <69.23%> (ø)`
...nsearch/flowframework/template/TemplateParser.java	`91.66% <91.66%> (ø)`
...pensearch/flowframework/workflow/WorkflowData.java	`100.00% <100.00%> (ø)`

Signed-off-by: Daniel Widdis <[email protected]>

dbwiddis · 2023-09-16T06:53:38Z

Merging #26 (136ceaa) into main (0f0b65d) will increase coverage by 74.26%.

I hereby claim the record for most code coverage improvement in this project ever!

joshpalis

Initial pass

src/main/java/demo/CreateIndexWorkflowStep.java

owaiskazi19

Thanks for putting this together. Did a thorough review this time :)

formatter/formatting.gradle

src/main/java/demo/CreateIndexWorkflowStep.java

src/main/java/org/opensearch/flowframework/template/ProcessNode.java

src/main/java/org/opensearch/flowframework/template/TemplateParser.java

owaiskazi19 · 2023-09-18T23:38:09Z

I see this PR fixes #19 as well but I don't see any RestHandler for the execution API. Am I missing something?

Signed-off-by: Daniel Widdis <[email protected]>

dbwiddis · 2023-09-19T00:21:38Z

I see this PR fixes #19 as well but I don't see any RestHandler for the execution API. Am I missing something?

It completes all the items listed in #19's description.

API in this context is not referring to a REST API

owaiskazi19 · 2023-09-19T00:24:06Z

API in this context is not referring to a REST API

Won't we need a REST API though for frontend/users to trigger the process sequencing which is already handled in this PR? Don't want to hold this PR but we should create a REST API for the execution. How about handling it in #19 and keeping the issue the open?

dbwiddis · 2023-09-19T00:31:44Z

Won't we need a REST API though for frontend to trigger the process sequencing which is already handled in this PR?

Yes, we'll need a REST call to send the template in that we can't do until we have the template design finalized. I see a few scattered REST API issues, is there a META with all of the required ones listed somewhere?

owaiskazi19 · 2023-09-19T00:34:45Z

Yes, we'll need a REST call to send the template in that we can't do until we have the template design finalized. I see a few scattered REST API issues, is there a META with all of the required ones listed somewhere?

I was under the impression #19 is for the REST execution API. Based on our decided design here we will need 2 APIs:

Execute API for frontend/users to trigger the worklow and pass use case template in the payload.
Orchestrate API to handle the operations to be done by the workflow.

I'll go ahead and approve the PR and we can have a new issue for the execution API.

owaiskazi19

Thanks for addressing the feedback patiently. LGTM!

dbwiddis · 2023-09-19T01:31:48Z

I was under the impression #19 is for the REST execution API. Based on our decided design here we will need 2 APIs:

Execute API for frontend/users to trigger the worklow and pass use case template in the payload.

Orchestrate API to handle the operations to be done by the workflow.

So exection is by orchestrate? That's what triggers the workflow using internal client stuff.

The template APIs would be CRUD.

We should definitely clarify all this in a new meta issue summarizing the various RFC comments.

owaiskazi19 · 2023-09-19T04:17:36Z

So exection is by orchestrate? That's what triggers the workflow using internal client stuff.

Execution is basically setting up the workflow which should be done by execution API. The operations to run on the workflow like Search/ingest would be done using orchestrate API.

The template APIs would be CRUD.

The execution API should be able to pass the created use case template from frontend to backend as payload.

src/main/java/org/opensearch/flowframework/template/ProcessNode.java

* Topological Sorting and Sequenced Execution Signed-off-by: Daniel Widdis <[email protected]> * Add javadocs Signed-off-by: Daniel Widdis <[email protected]> * Update demo to link to Workflow interface Signed-off-by: Daniel Widdis <[email protected]> * Replace System.out with logging Signed-off-by: Daniel Widdis <[email protected]> * Update with new interface signatures Signed-off-by: Daniel Widdis <[email protected]> * Demo passing input data at parse-time Signed-off-by: Daniel Widdis <[email protected]> * Demo passing data in between steps Signed-off-by: Daniel Widdis <[email protected]> * Change execute arg to list and refactor demo classes to own package Signed-off-by: Daniel Widdis <[email protected]> * Significantly simplify input/output data passing Signed-off-by: Daniel Widdis <[email protected]> * Add tests Signed-off-by: Daniel Widdis <[email protected]> * Fix javadocs and forbidden API issues Signed-off-by: Daniel Widdis <[email protected]> * Address code review comments Signed-off-by: Daniel Widdis <[email protected]> --------- Signed-off-by: Daniel Widdis <[email protected]> (cherry picked from commit a574f47) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Topological Sorting and Sequenced Execution (#26) * Topological Sorting and Sequenced Execution * Add javadocs * Update demo to link to Workflow interface * Replace System.out with logging * Update with new interface signatures * Demo passing input data at parse-time * Demo passing data in between steps * Change execute arg to list and refactor demo classes to own package * Significantly simplify input/output data passing * Add tests * Fix javadocs and forbidden API issues * Address code review comments --------- (cherry picked from commit a574f47) Signed-off-by: Daniel Widdis <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

dbwiddis mentioned this pull request Sep 8, 2023

Initial implementation of MLClient and workflow interface #25

Merged

owaiskazi19 reviewed Sep 8, 2023

View reviewed changes

dbwiddis mentioned this pull request Sep 11, 2023

[FEATURE] Improve Workflow api #29

Closed

4 tasks

dbwiddis added 8 commits September 13, 2023 15:12

Topological Sorting and Sequenced Execution

e0dcad0

Signed-off-by: Daniel Widdis <[email protected]>

Add javadocs

b837013

Signed-off-by: Daniel Widdis <[email protected]>

Update demo to link to Workflow interface

cc4cd64

Signed-off-by: Daniel Widdis <[email protected]>

Replace System.out with logging

6a87391

Signed-off-by: Daniel Widdis <[email protected]>

Update with new interface signatures

4b6a9fe

Signed-off-by: Daniel Widdis <[email protected]>

Demo passing input data at parse-time

d7211dc

Signed-off-by: Daniel Widdis <[email protected]>

Demo passing data in between steps

170e0ad

Signed-off-by: Daniel Widdis <[email protected]>

Change execute arg to list and refactor demo classes to own package

533431e

Signed-off-by: Daniel Widdis <[email protected]>

dbwiddis mentioned this pull request Sep 14, 2023

Adds Create Index Building block #38

Merged

dbwiddis added 2 commits September 14, 2023 17:06

Significantly simplify input/output data passing

10c09d6

Signed-off-by: Daniel Widdis <[email protected]>

Add tests

dfcd891

Signed-off-by: Daniel Widdis <[email protected]>

kotwanikunal reviewed Sep 15, 2023

View reviewed changes

src/main/java/org/opensearch/flowframework/template/ProcessNode.java Show resolved Hide resolved

dbwiddis requested a review from kotwanikunal September 16, 2023 00:21

dbwiddis marked this pull request as ready for review September 16, 2023 00:22

dbwiddis requested review from joshpalis, ohltyler, amitgalitz and jackiehanyang as code owners September 16, 2023 00:22

Fix javadocs and forbidden API issues

136ceaa

Signed-off-by: Daniel Widdis <[email protected]>

This was referenced Sep 16, 2023

[FEATURE] Create factory class for Workflow Steps #41

Closed

[FEATURE] Make Thread Pool available to Process Node #42

Closed

dbwiddis requested a review from owaiskazi19 September 16, 2023 05:38

joshpalis reviewed Sep 18, 2023

View reviewed changes

src/main/java/demo/CreateIndexWorkflowStep.java Show resolved Hide resolved

src/main/java/demo/CreateIndexWorkflowStep.java Show resolved Hide resolved

src/main/java/demo/CreateIndexWorkflowStep.java Show resolved Hide resolved

owaiskazi19 reviewed Sep 18, 2023

View reviewed changes

dbwiddis mentioned this pull request Sep 18, 2023

[DISCUSS] How should we parse the Template JSON? #46

Closed

Address code review comments

d1cbcbd

Signed-off-by: Daniel Widdis <[email protected]>

owaiskazi19 approved these changes Sep 19, 2023

View reviewed changes

jackiehanyang reviewed Sep 19, 2023

View reviewed changes

src/main/java/org/opensearch/flowframework/template/ProcessNode.java Show resolved Hide resolved

dbwiddis added the backport 2.x backport PRs to 2.x branch label Sep 19, 2023

joshpalis approved these changes Sep 19, 2023

View reviewed changes

dbwiddis mentioned this pull request Sep 19, 2023

Add WorkflowStep Factory and implement XContent-based Template Parsing #47

Merged

dbwiddis merged commit a574f47 into opensearch-project:main Sep 19, 2023
1 check passed

dbwiddis deleted the topo-sort branch September 19, 2023 17:51

opensearch-trigger-bot bot mentioned this pull request Sep 19, 2023

[Backport 2.x] Topological Sorting and Sequenced Execution #48

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Topological Sorting and Sequenced Execution #26

Topological Sorting and Sequenced Execution #26

dbwiddis commented Sep 8, 2023 •

edited

Loading

owaiskazi19 left a comment

codecov bot commented Sep 16, 2023 •

edited

Loading

dbwiddis commented Sep 16, 2023

joshpalis left a comment

owaiskazi19 left a comment

owaiskazi19 commented Sep 18, 2023

dbwiddis commented Sep 19, 2023

owaiskazi19 commented Sep 19, 2023 •

edited

Loading

dbwiddis commented Sep 19, 2023

owaiskazi19 commented Sep 19, 2023 •

edited

Loading

owaiskazi19 left a comment

dbwiddis commented Sep 19, 2023

owaiskazi19 commented Sep 19, 2023 •

edited

Loading

Topological Sorting and Sequenced Execution #26

Topological Sorting and Sequenced Execution #26

Conversation

dbwiddis commented Sep 8, 2023 • edited Loading

Description

Issues Resolved

owaiskazi19 left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 16, 2023 • edited Loading

Codecov Report

dbwiddis commented Sep 16, 2023

joshpalis left a comment

Choose a reason for hiding this comment

owaiskazi19 left a comment

Choose a reason for hiding this comment

owaiskazi19 commented Sep 18, 2023

dbwiddis commented Sep 19, 2023

owaiskazi19 commented Sep 19, 2023 • edited Loading

dbwiddis commented Sep 19, 2023

owaiskazi19 commented Sep 19, 2023 • edited Loading

owaiskazi19 left a comment

Choose a reason for hiding this comment

dbwiddis commented Sep 19, 2023

owaiskazi19 commented Sep 19, 2023 • edited Loading

dbwiddis commented Sep 8, 2023 •

edited

Loading

codecov bot commented Sep 16, 2023 •

edited

Loading

owaiskazi19 commented Sep 19, 2023 •

edited

Loading

owaiskazi19 commented Sep 19, 2023 •

edited

Loading

owaiskazi19 commented Sep 19, 2023 •

edited

Loading